Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: Pass the master, worker and bootstrap ignition content to the terraform. #301

Closed
wants to merge 2 commits into from

Conversation

yifan-gu
Copy link
Contributor

Previously, the installer binary generates the ignitions for master
and worker nodes, and pass the filename to the terraform.

However, in the new installer binary, we will instead pass the content
of the ignition files to terraform.

Also adds an option to use the bootstrap ignition from the tfvars file directly.

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 21, 2018
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 21, 2018
@@ -68,12 +71,14 @@ func (c *ConfigGenerator) GenerateIgnConfig(clusterDir string) error {
c.appendCertificateAuthority(&ignCfg, ca)
c.embedAppendBlock(&ignCfg, "master", fmt.Sprintf("etcd_index=%d", i))

if err = ignCfgToFile(ignCfg, filepath.Join(clusterDir, fmt.Sprintf(config.IgnitionPathMaster, i))); err != nil {
return err
masterIgn, err := json.MarshalIndent(&ignCfg, "", " ")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't be indenting as it will be part of the terraform.tfvars

IgnitionBootstrap string `json:"openshift_ignition_bootstrap,omitempty" yaml:"-"`
IgnitionMasters []string `json:"openshift_ignition_master,omitempty" yaml:"-"`
IgnitionWorker string `json:"openshift_ignition_worker,omitempty" yaml:"-"`
IgnitionBootstrap string `json:"ignition_bootstrap,omitempty" yaml:"ignitionBootstrap"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed yaml:"-" to yaml:"<something>"; We might wanna keep it ignored.

@yifan-gu yifan-gu force-pushed the ignition_by_value branch 7 times, most recently from d499821 to f759287 Compare September 21, 2018 20:12
@crawford
Copy link
Contributor

Can you fix up the commit message on the one? *.*: is a strange scope. *: would be more appropriate.

@yifan-gu yifan-gu force-pushed the ignition_by_value branch 2 times, most recently from d9c525d to adb9063 Compare September 21, 2018 21:18
@yifan-gu yifan-gu changed the title *.*: Pass the master, worker and bootstrap ignition content to the terraform. *: Pass the master, worker and bootstrap ignition content to the terraform. Sep 21, 2018
Previously, the installer binary generates the ignitions for master
and worker nodes, and pass the filename to the terraform.

However, in the new installer binary, we will instead pass the content
of the ignition files to terraform.
@crawford
Copy link
Contributor

This requires openshift/release#1630.

If the "ignition_bootstrap" is non-empty in the terraform.tfvars,
then use it as the content for the bootstrap ignition.

This will enable the new installer binary to launch a cluster.
@crawford
Copy link
Contributor

/retest

@crawford
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crawford, yifan-gu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot
Copy link
Contributor

@yifan-gu: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws 2e340f1 link /test e2e-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@wking wking added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 23, 2018
@crawford
Copy link
Contributor

(Accidentally) superseded by #289.

/close

@openshift-ci-robot
Copy link
Contributor

@crawford: Closing this PR.

In response to this:

(Accidentally) superseded by #289.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@yifan-gu yifan-gu deleted the ignition_by_value branch September 24, 2018 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants